fix(scene): reuse unchanged VLM inference - #231
Conversation
github-actions
Bot
added
type:bug
|
@robonix-ci test |
✅ robonix CI — 16/16 passed (100%)
LLM analysis Change summary: Added CI job 'scene-vlm-cache' to run pytest on system/scene/tests/test_vlm_inference_cache.py; Added vision_cache.py with FrameFingerprint, InferenceCounters, fingerprint_jpeg, fingerprint_bgr, frames_equivalent; Modified perception_vlm.py to add frame change threshold, failure backoff, and caching of last successful detections. Report: open HTML report. |
HeartLinked
left a comment
HeartLinked
left a comment
There was a problem hiding this comment.
Nice direction — the caching and retry accounting are a real improvement. Two things look blocking to me, though, because both reduce perception coverage on a healthy system under default settings.
1. The default frame-change threshold blinds the visual tier to small objects
frames_equivalent uses a whole-frame RMS over a 32x32 sample, so a local change is diluted by area (rms ~= sqrt(area_fraction) * delta / 255). Measured on 640x480 with this branch's own vision_cache, threshold=0.04:
32x32 px object rms=0.0285 SKIPPED
40x40 px object rms=0.0378 SKIPPED
48x48 px object rms=0.0469 inferred
Driving the real VLMObjectDetector._tick(): robot parked, a 40x40 px cup appears on a desk -> zero model calls over 150 ticks (~10 min). _DETECTION_PROMPT asks for cup / bottle / book / tool, which at ~2 m are exactly 40-60 px at 640x480.
Meanwhile the artefacts the threshold exists to absorb measure far below it — the 32x32 downsample already removes them:
q88 -> q60 re-encode rms=0.0027 sensor noise sigma=10 rms=0.0031
default threshold 0.0400
And there is no maximum cache age: _last_success_frame only changes on a successful inference, so this is permanent suppression, not a delay.
Suggested: default ~0.005-0.01, and/or compare the max per-block RMS instead of the frame mean; plus a bounded SCENE_VLM_CACHE_MAX_AGE_SEC so a mis-tuned threshold degrades into a delay rather than blindness.
2. A parseable answer whose edges are all filtered is booked as an endpoint failure
edges = parse_image_relations(raw, box_to_oid)
if raw_edges and not edges:
self._record_failure(self._clock())
return []near is deliberately outside IMAGE_RELATION_VOCAB (it is in the text path's RELATION_TYPES), and _normalize_relation maps no synonyms. So a healthy endpoint answering {"edges":[{"source":1,"target":2,"relation":"near"}]} counts as a failure. Reproduced at the 30 s rebuild cadence:
t= 0s failed=1 retry_in=30s
t= 90s failed=3 retry_in=120s
t=330s failed=4 retry_in=300s <- pinned at the cap, never recovers
Three effects stack: the backoff never clears (_last_success_edges is never written, so the cache can't short-circuit it); returning [] instead of None makes builder.py skip the text fallback, which does support near; and with MAX_STALE_ROUNDS=2 / INTERVAL_SEC=30 the semantic edges empty out after ~60 s. On dev the same response is just an authoritative empty round with no backoff, so this is a regression.
Suggested: only transport/parse failures (HTTP error, exception, non-JSON, missing or non-list edges) should call _record_failure; "parsed but no in-vocabulary edges" should count as processed and cache []. Same for _call_vlm, where any(not isinstance(item, dict) ...) discards a whole batch and enters backoff over one bad element — a per-item filter would be closer to the previous behaviour.
Checked and fine
edge.method = "cached" is safe (method is only read at builder.py:363 and in store.py); the deepcopy discipline is correct and _project_to_world doesn't mutate its input; not refreshing _last_success_frame on a cache hit correctly avoids a slow-drift ratchet; the relation signature plus sorted(nodes, key=object_id) is suitably conservative; _rgb_jpeg's tuple return has only one consumer; Pillow/numpy are already declared deps.
Minor, non-blocking: hub.latest stamps with local arrival time (_LatestSlot.write uses time.time()), not a source timestamp, so the comments are slightly off; cached edges keep their original updated_at; rounding T_cam_map to 1e-3 may make the relation cache hit far less often on real hardware than in the tests.
Happy to re-review once 1 and 2 are addressed.
|
@HeartLinked Thanks for the detailed measurements. Addressed in
The non-blocking points are covered too: Scene passes the hub delivery count instead of describing the local arrival stamp as a source timestamp; cached edges refresh Validation: 50 passed in the focused Scene suite, plus Ruff E9/F/I, compileall, diff-check, and authorship checks. |
|
CI follow-up: the failed Rust job was unrelated to the Scene diff. Rust 1.98 introduced I applied the behavior-preserving Clippy fix in
All passed locally. The new fork workflows are now awaiting maintainer approval ( |
|
@HeartLinked @enkerewpo Conflict resolved in |
3 participants
Problem
Scene repeatedly spent visual-model calls on unchanged inputs. The visual-tier detector polled the same cached JPEG without checking for meaningful change, and the image-grounded relation pass reran against stable visible objects, geometry, and camera state. Failures retried on the normal worker cadence, so endpoint outages could consume the shared model-call budget quickly.
Implementation
SCENE_VLM_CACHE_MAX_AGE_SEC(default 120 seconds);0disables reuse across new deliveries.Validation
python3 -S -m pytest -q system/scene/tests/test_scene_graph.py system/scene/tests/test_vlm_inference_cache.py system/scene/tests/test_perception_vlm_geometry.py system/scene/tests/test_image_relations.pyruff check --select E9,F,Ion the changed cache, detector, relation, builder, and regression-test modules.python3 -m compileall -q system/scene/scene_service system/scene/tests/test_vlm_inference_cache.py system/scene/tests/test_image_relations.pygit diff --checkpython3 scripts/check_commit_authorship.py --base origin/dev --head HEADCompatibility
(JPEG, float timestamp)fetchers remain accepted; the Scene service now passes the hub delivery count when available.Fixes #207